Rehydrate components with thisObject pointing to the script#304
Rehydrate components with thisObject pointing to the script#304Willem1987 wants to merge 1 commit intojenkinsci:masterfrom
Conversation
src/test/groovy/com/lesfurets/jenkins/unit/declarative/TestDeclarativePipeline.groovy
Outdated
Show resolved
Hide resolved
a6df9ae to
722d5b8
Compare
|
Unfortunately this works locally. Any idea how to reproduce? |
|
It fails locally for me. Here's the stacktrace I get from running the unit tests in IntelliJ: I would suggest making a fresh checkout with this branch or running |
c44f199 to
e757d53
Compare
|
I have overwritten my fork of master with upstream master and re-applied commit to that. Then i force pushed the state to the existing branch. I tried the git clean -dffx but locally i could not get it to break. |
|
The appveyor seems to run the test and work? Could it be a jdk thing or specific environment? I tried this test on a windows (virtual workstation) and macbook. |
|
I changed from oracle jdk 8 to openjdk 8 which i thought to have seen in the travis build. openjdk version "1.8.0_265" |
e757d53 to
bf70041
Compare
|
Okay now im lost. I renamed the jenkinsfile to Declarative_thisObject_JenkinsFile and the test to jenkinsfile_thisObject and now it seems to work? So somehow this in a filename might cause problems? Or something else fixed it |
src/test/groovy/com/lesfurets/jenkins/unit/declarative/TestDeclarativePipeline.groovy
Show resolved
Hide resolved
stchar
left a comment
There was a problem hiding this comment.
Please improve tests. assertJobStatusSucces() is not enough IMHO
e12cdd3 to
4b7ab37
Compare
stchar
left a comment
There was a problem hiding this comment.
Could you please add few more changes in tests and I would be glad to merge it
|
|
||
| String testThis(Script scriptRef, String text){ | ||
| echo "This is a script?: ${this == scriptRef}" | ||
| return text |
There was a problem hiding this comment.
return "$text: This is a script?: ${this == scriptRef}"
Would allow to:
assertCallStack().contains("pipeline unit tests completed: This is a script?: true")
So you can help other contributors to detect broken places in the code in later updates
There was a problem hiding this comment.
Then we could only call the method in 1 place. I called it from environment, options, when, trigger and script content now.
There was a problem hiding this comment.
Scratch that, it technically works, but the values returned would not be valid in real life. For test it is okay i think
There was a problem hiding this comment.
I updated it a little to pass in Object. So that this always works and we can check instanceof
There was a problem hiding this comment.
In addition to that i added an assert so that it may not contain false
eb26bee to
cb81df0
Compare
…l make this.sh work and make this in the pipeline script passable to methods.
cb81df0 to
dcbddce
Compare
|
@nre-ableton @stchar Is there anything i can do on this? I updated the MR according to comments i think. If i need to do anything let me know. I started using jenkinsPipelineUnit in our builds and would like the this ref feature. |
|
I'll let @stchar take things from here, since he's been the primary reviewer on this PR. |
@nre-ableton Though i understand this viewpoint, it has been months now. Is he still active? Is there anything i can do to further progress this? |
My understanding is that he's recently become a father, so I expect he's pretty busy (and also possibly on paternity leave). Probably doesn't hurt to ping him again (hi @stchar!), but I don't know when he'll be back. |
|
Closing due too revised insights from #299 |
Attempt to fix #303